-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Move key changes into FormulaRuntime. #333
Conversation
fb905d4
to
0e26da1
Compare
JaCoCo Code Coverage 78.72% ✅
Generated by 🚫 Danger |
) : ManagerDelegate { | ||
private val implementation = formula.implementation() | ||
private var manager: FormulaManagerImpl<Input, *, Output>? = null | ||
private var hasInitialFinished = false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not needed anymore - previously it was used to ensure that on formula start, we wait for all actions to start running before emitting a value. Currently, it's done within FormulaManagerImpl
.
threadChecker.check("Input arrived on a wrong thread.") | ||
if (!runtime.isKeyValid(input)) { | ||
runtime.terminate() | ||
runtime = runtimeFactory() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was a bit weird that we recreate the runtime if key changes. Instead, the runtime will handle state reset internally.
performTermination() | ||
terminateManager(manager) | ||
|
||
if (localInputId != inputId && !terminated) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic here looks fine, but it's a little confusing that we have both manager.isTerminated()
and a terminated
flag (and that they'll have opposite values).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They could potentially both be true at the same time, will add documentation and maybe rename it.
b27b7a3
to
b0b40c7
Compare
b0b40c7
to
66a7c63
Compare
What
To remove duplication within RxJavaRuntime and FlowRuntime, I'm moving logic into shared
FormulaRuntime
class